fix(query-insights): traverse shard branches in planner-tree helpers#703
fix(query-insights): traverse shard branches in planner-tree helpers#703hanhan761 wants to merge 2 commits into
Conversation
fix(query-insights): traverse shard branches in planner-tree helpers `findStageInPlan()` and `collectPlannerStages()` only traversed inputStage and inputStages — they missed stages nested under shards structures in sharded cluster explain plans. This meant planner-stage signals like isBitmap (bitmap index) and low-cardinality index detection could be silently missed. - Extend findStageInPlan() to recurse through shards arrays (matching traverseStage / checkStageForSort behavior). - Extend collectPlannerStages() similarly. - Add two sharded test cases: IXSCAN with isBitmap under a SHARD_MERGE > shards > SINGLE_SHARD path, and a mixed-case where planner is sharded but exec stats are flat. Closes microsoft#620 @
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds shard-branch traversal support to query insight analysis so stage detection works correctly for sharded explain plans.
Changes:
- Recurse into
shardsbranches when flattening planner stages. - Recurse into
shardsbranches when searching for a stage in a plan tree. - Add tests covering shard-branch scenarios for bitmap index detection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/documentdb/queryInsights/StagePropertyExtractor.ts | Extends planner-stage collection logic to traverse sharded branches. |
| src/documentdb/queryInsights/ExplainPlanAnalyzer.ts | Extends stage search to recurse into shard branches. |
| src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts | Adds regression tests for shard-branch bitmap stage detection. |
| if (stage.shards && Array.isArray(stage.shards)) { | ||
| for (const shard of stage.shards) { | ||
| this.collectPlannerStages(shard as Document, accumulator); | ||
| } | ||
| } |
| if (plan.shards && Array.isArray(plan.shards)) { | ||
| for (const shard of plan.shards) { | ||
| const found = this.findStageInPlan(shard as Document, stageName); | ||
| if (found) { | ||
| return found; | ||
| } | ||
| } | ||
| } |
| }); | ||
|
|
||
| it('detects IXSCAN in planner shards even when exec stats use flat structure', () => { | ||
| // Edge case: planner is sharded, execution stats may not be |
tnaum-ms
added
on-hold
|
Thanks for this one. The shard branch traversal is a real gap and the direction here is right. We are putting this on hold and scheduling it for the 0.10.1 release. The reason is testing. Doing this correctly means handling the different shapes of sharded explain output across platforms, and the current tests use simplified fixtures that do not match real sharded explain output, so we cannot validate it with confidence yet. We want a larger test setup before we commit to the change. We expect to have that after our MongoDB Atlas discovery work ships, which will let us test against a wider range of platforms. We have marked the PR as on hold and moved it to the 0.10.1 milestone, and we will pick it back up then. Appreciate your patience. |
Problem
findStageInPlan()andcollectPlannerStages()only traversedinputStageandinputStages— they missed stages nested undershardsstructures in sharded cluster explain plans. This meant planner-stage signals likeisBitmap(bitmap index detection) could be silently missed for sharded DocumentDB clusters.In contrast,
traverseStages()(execution-stage traversal for UI) already handledshards.Fix
findStageInPlan()(ExplainPlanAnalyzer.ts): addedshardsarray recursion, matching the pattern used bytraverseStage()andcheckStageForSort()elsewhere in the same file.collectPlannerStages()(StagePropertyExtractor.ts): addedshardsarray recursion, matchingtraverseStages()in the same class.Tests
Two new test cases added:
SHARD_MERGE → shards → SINGLE_SHARD → FETCH → IXSCANAll 15 existing + 2 new tests pass. ✅
Closes #620
🤖 Generated with Claude Code